Skip to content

Add proxy and user_agent labels to user login counter metric - #60239

Merged
vapopov merged 9 commits into
masterfrom
vapopov/user-login-agent-proxy-tags-metric
Nov 20, 2025
Merged

Add proxy and user_agent labels to user login counter metric#60239
vapopov merged 9 commits into
masterfrom
vapopov/user-login-agent-proxy-tags-metric

Conversation

@vapopov

@vapopov vapopov commented Oct 14, 2025

Copy link
Copy Markdown
Contributor

In this PR, added a user agent to the client tools request header. During login, it is passed through the proxy to the auth service. Additionally, information about the public proxy address is included so it can be recorded in metrics.

The user_login_total counter now has two labels: proxy and user_agent.

  • The proxy_group_id label allows us to track which regions are used by customers during login.
  • The user_agent_type and version label helps identify the client tools version in use, to detect any affected versions in future, and monitor the progress of client tools managed updates.

Example:

# HELP user_login_per_client Number of times there was a user login with specific client version and GroupID that routes request
# TYPE user_login_per_client counter
user_login_per_client{proxy_group_id="proxy1",user_agent_type="web",version="18.2.5"} 1
user_login_per_client{proxy_group_id="proxy1",user_agent_type="api",version="18.2.5"} 1
user_login_per_client{proxy_group_id="proxy2",user_agent_type="api",version="18.2.5"} 2

Related:

@tigrato

tigrato commented Oct 14, 2025

Copy link
Copy Markdown
Contributor

How can we ensure that an attacker doesn't break cloud monitoring by spamming tons of agents and causing Prometheus to fail? Are we going to drop them in cloud?

@rosstimothy rosstimothy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Tiago, this seems like it could be a very high cardinality metric. cc @evanfreed.

Comment thread integrations/event-handler/go.mod
Comment thread lib/auth/sessions.go Outdated
Comment thread lib/auth/sessions.go Outdated
Comment thread lib/client/weblogin.go Outdated

@sclevine sclevine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the concern about high cardinality on the user agent is worth addressing for scalability reasons, even if authenticated users could likely already cause trouble by connecting many agent versions (which we already record).

Could we parse the user agent for Teleport major version, and only accept reasonable values?

Comment thread lib/auth/sessions.go Outdated
@vapopov

vapopov commented Oct 14, 2025

Copy link
Copy Markdown
Contributor Author

How can we ensure that an attacker doesn't break cloud monitoring by spamming tons of agents and causing Prometheus to fail? Are we going to drop them in cloud?

@tigrato in order to do that, you need a user from the cluster and must complete the login process. We would probably need to reset counters periodically as well

Replaced Sprintf with string concatenation
@vapopov
vapopov force-pushed the vapopov/user-login-agent-proxy-tags-metric branch from cbec0d8 to 3cc147e Compare October 15, 2025 05:34
@vapopov

vapopov commented Oct 31, 2025

Copy link
Copy Markdown
Contributor Author

I’ve added a periodic cleanup (hourly) and implemented the label separation by type and version as proposed by @evanfreed.
Since the metric is periodically reset, I’ve introduced a separate metric, user_login_per_client, to avoid breaking existing dashboards built on user_login_total.

@vapopov

vapopov commented Nov 7, 2025

Copy link
Copy Markdown
Contributor Author

Any other suggestions or concerns regarding this PR? cc: @tigrato @rosstimothy @evanfreed @hugoShaka

Comment thread lib/web/apiserver.go Outdated
Comment thread lib/auth/auth.go Outdated
@rosstimothy

Copy link
Copy Markdown
Contributor

@evanfreed do you have any concerns about consuming this metric in Cloud?

Login counter metric should not be exported.
@evanfreed

Copy link
Copy Markdown
Contributor

@evanfreed do you have any concerns about consuming this metric in Cloud?

With the recent adjustments we should be good. Appreciate keeping an eye out for that.

@vapopov

vapopov commented Nov 17, 2025

Copy link
Copy Markdown
Contributor Author

@rosstimothy could you please take a look on addressed changes aa9f142

Comment thread lib/auth/auth.go Outdated
Comment thread lib/auth/auth.go Outdated
Comment thread lib/auth/auth.go Outdated
Comment thread lib/web/apiserver.go Outdated
@rosstimothy
rosstimothy self-requested a review November 18, 2025 14:17
Comment thread lib/auth/auth.go Outdated
Comment thread lib/auth/auth.go Outdated
Comment thread lib/auth/auth.go Outdated
@vapopov
vapopov requested a review from rosstimothy November 19, 2025 18:27
@vapopov
vapopov force-pushed the vapopov/user-login-agent-proxy-tags-metric branch from c074d8f to 54a8560 Compare November 19, 2025 18:44
@vapopov
vapopov force-pushed the vapopov/user-login-agent-proxy-tags-metric branch from 54a8560 to cb31dbc Compare November 19, 2025 18:45
@vapopov
vapopov added this pull request to the merge queue Nov 20, 2025
Merged via the queue into master with commit e2d968e Nov 20, 2025
45 checks passed
@vapopov
vapopov deleted the vapopov/user-login-agent-proxy-tags-metric branch November 20, 2025 09:58
@backport-bot-workflows

Copy link
Copy Markdown
Contributor

@vapopov See the table below for backport results.

Branch Result
branch/v17 Failed
branch/v18 Create PR

vapopov added a commit that referenced this pull request Nov 20, 2025
* Added user login counter metrics labels with proxy information and user-agent version

* Fixed defining default user-agent when `extraHeaders` map was nil
Replaced Sprintf with string concatenation

* Fix the env proxy group ID

* Add periodic clean up and change label for the metric

* Use `ProxyGroupID` from the proxy configuration.
Login counter metric should not be exported.

* Namespace new metric
CR changes

* Removed global constants and changed help message

* Comments adjustment
github-merge-queue Bot pushed a commit that referenced this pull request Dec 4, 2025
…#61622)

* Added user login counter metrics labels with proxy information and user-agent version

* Fixed defining default user-agent when `extraHeaders` map was nil
Replaced Sprintf with string concatenation

* Fix the env proxy group ID

* Add periodic clean up and change label for the metric

* Use `ProxyGroupID` from the proxy configuration.
Login counter metric should not be exported.

* Namespace new metric
CR changes

* Removed global constants and changed help message

* Comments adjustment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v17 backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants